Add parser for arbitrary expressions

The expressions can be used in if 'test' and in the
right side of assignements.

The expressions can contain any number of sub-expressions
combined by either arithmetic operators, comparison operators,
or boolean operators.

Random Usage Examples:
$result = ((( $two + 2) / $one) + 4 * 5.45) - (6 << 7) + (0x800 + -9)
or
if ($a < 10) and ($a + 10 != 200) {
...
}

CHANGELOG

@@ -1,4 +1,6 @@
 0.24.x
+    Fixed #1585 - Allow complex 'if' and variable expressions
+
     Fixed #1564 - Saving File#checksum to state.yaml broken
 
     Fixed #1603 - Added support for running Puppet inside a Rack application

lib/puppet/parser/grammar.ra

@@ -9,7 +9,19 @@ token FALSE EQUALS APPENDS LESSEQUAL NOTEQUAL DOT COLON LLCOLLECT RRCOLLECT
 token QMARK LPAREN RPAREN ISEQUAL GREATEREQUAL GREATERTHAN LESSTHAN
 token IF ELSE IMPORT DEFINE ELSIF VARIABLE CLASS INHERITS NODE BOOLEAN
 token NAME SEMIC CASE DEFAULT AT LCOLLECT RCOLLECT CLASSNAME CLASSREF
-token NOT OR AND UNDEF PARROW
+token NOT OR AND UNDEF PARROW PLUS MINUS TIMES DIV LSHIFT RSHIFT UMINUS
+
+prechigh
+    right NOT
+    nonassoc UMINUS
+    left  TIMES DIV
+    left  MINUS PLUS
+    left  LSHIFT RSHIFT
+    left  NOTEQUAL ISEQUAL
+    left  GREATEREQUAL GREATERTHAN LESSTHAN LESSEQUAL
+    left  AND
+    left  OR
+preclow
 
 rule
 program:    statements {
@@ -283,7 +295,7 @@ resourcename:   quotedtext
             | variable
             | array
 
-assignment:     VARIABLE EQUALS rvalue {
+assignment:     VARIABLE EQUALS expression {
     if val[0] =~ /::/
         raise Puppet::ParseError, "Cannot assign to variables in other namespaces"
     end
@@ -292,7 +304,7 @@ assignment:     VARIABLE EQUALS rvalue {
     result = ast AST::VarDef, :name => variable, :value => val[2]
 }
 
-append:     VARIABLE APPENDS rvalue {
+append:     VARIABLE APPENDS expression {
     variable = ast AST::Name, :value => val[0]
     result = ast AST::VarDef, :name => variable, :value => val[2], :append => true
 }
@@ -395,7 +407,7 @@ resourceref: NAME LBRACK rvalue RBRACK {
     result = ast AST::ResourceReference, :type => val[0], :title => val[2]
 }
 
-ifstatement:      IF iftest LBRACE statements RBRACE else {
+ifstatement:      IF expression LBRACE statements RBRACE else {
     args = {
         :test => val[1],
         :statements => val[3]
@@ -413,9 +425,70 @@ else:             # nothing
     result = ast AST::Else, :statements => val[2]
 }
 
-# Currently we only support a single value, but eventually one assumes
-# we'll support operators and such.
-iftest:         rvalue
+# Unlike yacc/bison, it seems racc
+# gives tons of shift/reduce warnings
+# with the following syntax:
+# 
+# expression: ...
+#            | expression arithop expressio { ... }
+#            
+# arithop: PLUS | MINUS | DIVIDE | TIMES ...
+#
+# So I had to develop the expression by adding one rule
+# per operator :-(
+
+expression:   rvalue
+            | expression PLUS expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression MINUS expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression DIV expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression TIMES expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression LSHIFT expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression RSHIFT expression {
+    result = ast AST::ArithmeticOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | MINUS expression =UMINUS {
+    result = ast AST::Minus, :value => val[1]
+}
+            | expression NOTEQUAL expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression ISEQUAL expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression GREATERTHAN expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression GREATEREQUAL expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression LESSTHAN expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | expression LESSEQUAL expression {
+    result = ast AST::ComparisonOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}
+            | NOT expression {
+    result = ast AST::Not, :value => val[1]
+}
+            | expression AND expression {
+    result = ast AST::BooleanOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}            
+            | expression OR expression {
+    result = ast AST::BooleanOperator, :operator => val[1], :lval => val[0], :rval => val[2]
+}            
+            | LPAREN expression RPAREN {
+    result = val[1]
+}
 
 casestatement:  CASE rvalue LBRACE caseopts RBRACE {
     options = val[3]

lib/puppet/parser/lexer.rb

@@ -126,12 +126,22 @@ class Puppet::Parser::Lexer
         '\\' => :BACKSLASH,
         '=>' => :FARROW,
         '+>' => :PARROW,
+        '+' => :PLUS,
+        '-' => :MINUS,
+        '/' => :DIV,
+        '*' => :TIMES,
+        '<<' => :LSHIFT,
+        '>>' => :RSHIFT,
         %r{([a-z][-\w]*::)+[a-z][-\w]*} => :CLASSNAME,
         %r{((::){0,1}[A-Z][-\w]*)+} => :CLASSREF
     )
 
     TOKENS.add_tokens "Whatever" => :DQTEXT, "Nomatter" => :SQTEXT, "alsonomatter" => :BOOLEAN
 
+    TOKENS.add_token :NUMBER, %r{\b(?:0[xX][0-9A-Fa-f]+|0?\d+(?:\.\d+)?(?:[eE]-?\d+)?)\b} do |lexer, value|
+        [TOKENS[:NAME], value]
+    end
+
     TOKENS.add_token :NAME, %r{[a-z0-9][-\w]*} do |lexer, value|
         string_token = self
         # we're looking for keywords here
@@ -145,10 +155,6 @@ class Puppet::Parser::Lexer
         [string_token, value]
     end
 
-    TOKENS.add_token :NUMBER, %r{[0-9]+} do |lexer, value|
-        [TOKENS[:NAME], value]
-    end
-
     TOKENS.add_token :COMMENT, %r{#.*}, :skip => true
 
     TOKENS.add_token :RETURN, "\n", :skip => true, :incr_line => true, :skip_text => true

spec/unit/parser/lexer.rb

@@ -136,7 +136,13 @@ describe Puppet::Parser::Lexer::TOKENS do
         :BACKSLASH => '\\',
         :FARROW => '=>',
         :PARROW => '+>',
-        :APPENDS => '+='
+        :APPENDS => '+=',
+        :PLUS => '+',
+        :MINUS => '-',
+        :DIV => '/',
+        :TIMES => '*',
+        :LSHIFT => '<<',
+        :RSHIFT => '>>',
     }.each do |name, string|
         it "should have a token named #{name.to_s}" do
             Puppet::Parser::Lexer::TOKENS[name].should_not be_nil
@@ -213,10 +219,34 @@ describe Puppet::Parser::Lexer::TOKENS[:NAME] do
 end
 
 describe Puppet::Parser::Lexer::TOKENS[:NUMBER] do
-    before { @token = Puppet::Parser::Lexer::TOKENS[:NUMBER] }
+    before do
+        @token = Puppet::Parser::Lexer::TOKENS[:NUMBER]
+#        @regex = Regexp.new('^'+@token.regex.source+'$')
+        @regex = @token.regex
+    end
 
     it "should match against numeric terms" do
-        @token.regex.should =~ "2982383139"
+        @regex.should =~ "2982383139"
+    end
+
+    it "should match against float terms" do
+        @regex.should =~ "29823.235"
+    end
+
+    it "should match against hexadecimal terms" do
+        @regex.should =~ "0xBEEF0023"
+    end
+
+    it "should match against float with exponent terms" do
+        @regex.should =~ "10e23"
+    end
+
+    it "should match against float terms with negative exponents" do
+        @regex.should =~ "10e-23"
+    end
+
+    it "should match against float terms with fractional parts and exponent" do
+        @regex.should =~ "1.234e23"
     end
 
     it "should return the NAME token and the value" do

spec/unit/parser/parser.rb

@@ -8,6 +8,7 @@ describe Puppet::Parser do
 
     before :each do
         @parser = Puppet::Parser::Parser.new :environment => "development"
+        @true_ast = AST::Boolean.new :value => true
     end
 
     describe "when parsing append operator" do
@@ -31,4 +32,49 @@ describe Puppet::Parser do
         end
 
     end
-end
+
+    describe Puppet::Parser, "when parsing 'if'" do
+        it "not, it should create the correct ast objects" do
+            AST::Not.expects(:new).with { |h| h[:value].is_a?(AST::Boolean) }
+            @parser.parse("if ! true { $var = 1 }")
+        
+        end
+
+        it "boolean operation, it should create the correct ast objects" do
+            AST::BooleanOperator.expects(:new).with { 
+                |h| h[:rval].is_a?(AST::Boolean) and h[:lval].is_a?(AST::Boolean) and h[:operator]=="or"
+            }
+            @parser.parse("if true or true { $var = 1 }")
+
+        end
+
+        it "comparison operation, it should create the correct ast objects" do
+             AST::ComparisonOperator.expects(:new).with { 
+                 |h| h[:lval].is_a?(AST::Name) and h[:rval].is_a?(AST::Name) and h[:operator]=="<"
+             }
+             @parser.parse("if 1 < 2 { $var = 1 }")
+
+        end
+
+    end
+
+    describe Puppet::Parser, "when parsing if complex expressions" do
+         it "should create a correct ast tree" do
+             AST::ComparisonOperator.expects(:new).with { 
+                 |h| h[:rval].is_a?(AST::Name) and h[:lval].is_a?(AST::Name) and h[:operator]==">"
+             }.returns("whatever")
+             AST::ComparisonOperator.expects(:new).with { 
+                 |h| h[:rval].is_a?(AST::Name) and h[:lval].is_a?(AST::Name) and h[:operator]=="=="
+             }.returns("whatever")
+             AST::BooleanOperator.expects(:new).with {
+                 |h| h[:rval]=="whatever" and h[:lval]=="whatever" and h[:operator]=="and"                
+             }
+             @parser.parse("if (1 > 2) and (1 == 2) { $var = 1 }")
+         end
+
+         it "should raise an error on incorrect expression" do
+             lambda { @parser.parse("if (1 > 2 > ) or (1 == 2) { $var = 1 }") }.should raise_error
+        end
+
+     end
+ end

test/data/snippets/arithmetic_expression.pp

@@ -0,0 +1,8 @@
+
+$one = 1.30
+$two = 2.034e-2
+
+$result = ((( $two + 2) / $one) + 4 * 5.45) - (6 << 7) + (0x800 + -9)
+
+
+notice("result is $result == 1295.87692307692")

test/data/snippets/ifexpression.rb

@@ -0,0 +1,6 @@
+$one = 1
+$two = 2
+
+if ($one < $two) and (($two < 3) or ($two == 2)) {
+    notice("True!")
+}