Refactoring the AST classes just a bit. I realized that

all of the evaluate() methods only ever accepted a scope,
and sometimes one other option, so I switched them all to
use named arguments instead of a hash.

22 files changed, 68 insertions, 110 deletions

diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index cac18fef4..c9bd7c9e8 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -27,7 +27,7 @@ class Puppet::Parser::AST
     # should override this method.
     # of the contained children and evaluates them in turn, returning a
     # list of all of the collected values, rejecting nil values
-    def evaluate(args)
+    def evaluate(*options)
         raise Puppet::DevError, "Did not override #evaluate in %s" % self.class
     end
 
@@ -47,11 +47,11 @@ class Puppet::Parser::AST
     # correctly handles errors.  It is critical to use this method because
     # it can enable you to catch the error where it happens, rather than
     # much higher up the stack.
-    def safeevaluate(options)
+    def safeevaluate(*options)
         # We duplicate code here, rather than using exceptwrap, because this
         # is called so many times during parsing.
         begin
-            return self.evaluate(options)
+            return self.evaluate(*options)
         rescue Puppet::Error => detail
             raise adderrorcontext(detail)
         rescue => detail
diff --git a/lib/puppet/parser/ast/astarray.rb b/lib/puppet/parser/ast/astarray.rb
index 5f1e838d0..b66fd6bba 100644
--- a/lib/puppet/parser/ast/astarray.rb
+++ b/lib/puppet/parser/ast/astarray.rb
@@ -15,8 +15,7 @@ class Puppet::Parser::AST
         end
 
         # Evaluate our children.
-        def evaluate(hash)
-            scope = hash[:scope]
+        def evaluate(scope)
             rets = nil
             # We basically always operate declaratively, and when we
             # do we need to evaluate the settor-like statements first.  This
@@ -51,7 +50,7 @@ class Puppet::Parser::AST
             }
 
             rets = [settors, others].flatten.collect { |child|
-                child.safeevaluate(:scope => scope)
+                child.safeevaluate(scope)
             }
             return rets.reject { |o| o.nil? }
         end
diff --git a/lib/puppet/parser/ast/caseopt.rb b/lib/puppet/parser/ast/caseopt.rb
index d1d9d0e9c..824bde853 100644
--- a/lib/puppet/parser/ast/caseopt.rb
+++ b/lib/puppet/parser/ast/caseopt.rb
@@ -44,17 +44,17 @@ class Puppet::Parser::AST
         def eachvalue(scope)
             if @value.is_a?(AST::ASTArray)
                 @value.each { |subval|
-                    yield subval.evaluate(:scope => scope)
+                    yield subval.safeevaluate(scope)
                 }
             else
-                yield @value.evaluate(:scope => scope)
+                yield @value.safeevaluate(scope)
             end
         end
 
         # Evaluate the actual statements; this only gets called if
         # our option matched.
-        def evaluate(hash)
-            return @statements.safeevaluate(hash)
+        def evaluate(scope)
+            return @statements.safeevaluate(scope)
         end
     end
 end
diff --git a/lib/puppet/parser/ast/casestatement.rb b/lib/puppet/parser/ast/casestatement.rb
index 3c6f9c7e2..aa03090de 100644
--- a/lib/puppet/parser/ast/casestatement.rb
+++ b/lib/puppet/parser/ast/casestatement.rb
@@ -8,9 +8,8 @@ class Puppet::Parser::AST
 
         # Short-curcuit evaluation.  Return the value of the statements for
         # the first option that matches.
-        def evaluate(hash)
-            scope = hash[:scope]
-            value = @test.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            value = @test.safeevaluate(scope)
             sensitive = Puppet[:casesensitive]
             value = value.downcase if ! sensitive and value.respond_to?(:downcase)
 
@@ -30,7 +29,7 @@ class Puppet::Parser::AST
 
                 if found
                     # we found a matching option
-                    retvalue = option.safeevaluate(:scope => scope)
+                    retvalue = option.safeevaluate(scope)
                     break
                 end
 
@@ -42,7 +41,7 @@ class Puppet::Parser::AST
             # Unless we found something, look for the default.
             unless found
                 if default
-                    retvalue = default.safeevaluate(:scope => scope)
+                    retvalue = default.safeevaluate(scope)
                 else
                     Puppet.debug "No true answers and no default"
                     retvalue = nil
diff --git a/lib/puppet/parser/ast/collection.rb b/lib/puppet/parser/ast/collection.rb
index e05977a47..31f508929 100644
--- a/lib/puppet/parser/ast/collection.rb
+++ b/lib/puppet/parser/ast/collection.rb
@@ -9,11 +9,9 @@ class Collection < AST::Branch
     attr_accessor :type, :query, :form
 
     # We return an object that does a late-binding evaluation.
-    def evaluate(hash)
-        scope = hash[:scope]
-
+    def evaluate(scope)
         if self.query
-            str, code = self.query.safeevaluate :scope => scope
+            str, code = self.query.safeevaluate scope
         else
             str = code = nil
         end
diff --git a/lib/puppet/parser/ast/collexpr.rb b/lib/puppet/parser/ast/collexpr.rb
index 4a96d9c61..3e13d9400 100644
--- a/lib/puppet/parser/ast/collexpr.rb
+++ b/lib/puppet/parser/ast/collexpr.rb
@@ -9,9 +9,7 @@ class CollExpr < AST::Branch
     attr_accessor :test1, :test2, :oper, :form, :type, :parens
 
     # We return an object that does a late-binding evaluation.
-    def evaluate(hash)
-        scope = hash[:scope]
-
+    def evaluate(scope)
         # Make sure our contained expressions have all the info they need.
         [@test1, @test2].each do |t|
             if t.is_a?(self.class)
@@ -21,8 +19,8 @@ class CollExpr < AST::Branch
         end
 
         # The code is only used for virtual lookups
-        str1, code1 = @test1.safeevaluate :scope => scope
-        str2, code2 = @test2.safeevaluate :scope => scope
+        str1, code1 = @test1.safeevaluate scope
+        str2, code2 = @test2.safeevaluate scope
 
         # First build up the virtual code.
         # If we're a conjunction operator, then we're calling code.  I did
diff --git a/lib/puppet/parser/ast/definition.rb b/lib/puppet/parser/ast/definition.rb
index 3d6d6188c..2e39e7332 100644
--- a/lib/puppet/parser/ast/definition.rb
+++ b/lib/puppet/parser/ast/definition.rb
@@ -27,10 +27,7 @@ class Puppet::Parser::AST
             false
         end
 
-        def evaluate(options)
-            origscope = options[:scope]
-            resource = options[:resource]
-
+        def evaluate(origscope, resource)
             # Create a new scope.
             scope = subscope(origscope, resource)
 
@@ -49,7 +46,7 @@ class Puppet::Parser::AST
             set_resource_parameters(scope, resource)
 
             if self.code
-                return self.code.safeevaluate(:scope => scope)
+                return self.code.safeevaluate(scope)
             else
                 return nil
             end
@@ -181,7 +178,7 @@ class Puppet::Parser::AST
                     arg = symbolize(arg)
                     unless args.include?(arg)
                         if defined? default and ! default.nil?
-                            default = default.safeevaluate :scope => scope
+                            default = default.safeevaluate scope
                             args[arg] = default
                             #Puppet.debug "Got default %s for %s in %s" %
                             #    [default.inspect, arg.inspect, @name.inspect]
diff --git a/lib/puppet/parser/ast/else.rb b/lib/puppet/parser/ast/else.rb
index e76051372..affac625d 100644
--- a/lib/puppet/parser/ast/else.rb
+++ b/lib/puppet/parser/ast/else.rb
@@ -12,9 +12,8 @@ class Puppet::Parser::AST
 
         # Evaluate the actual statements; this only gets called if
         # our test was true matched.
-        def evaluate(hash)
-            scope = hash[:scope]
-            return @statements.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            return @statements.safeevaluate(scope)
         end
     end
 end
diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb
index 0cd1fff62..63d7c7abf 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -7,18 +7,11 @@ class Puppet::Parser::AST
 
         @settor = true
 
-        def evaluate(hash)
+        def evaluate(scope)
             # We don't need to evaluate the name, because it's plaintext
+            args = @arguments.safeevaluate(scope)
 
-            # Just evaluate the arguments
-            scope = hash[:scope]
-
-            args = @arguments.safeevaluate(:scope => scope)
-
-            #exceptwrap :message => "Failed to execute %s" % @name,
-            #        :type => Puppet::ParseError do
-                return scope.send("function_" + @name, args)
-            #end
+            return scope.send("function_" + @name, args)
         end
 
         def initialize(hash)
diff --git a/lib/puppet/parser/ast/hostclass.rb b/lib/puppet/parser/ast/hostclass.rb
index 63900d0e3..42d13f4b2 100644
--- a/lib/puppet/parser/ast/hostclass.rb
+++ b/lib/puppet/parser/ast/hostclass.rb
@@ -20,9 +20,7 @@ class Puppet::Parser::AST
         end
 
         # Evaluate the code associated with this class.
-        def evaluate(options)
-            scope = options[:scope]
-            raise(ArgumentError, "Classes require resources") unless options[:resource]
+        def evaluate(scope, resource)
             # Verify that we haven't already been evaluated.  This is
             # what provides the singleton aspect.
             if existing_scope = scope.compile.class_scope(self)
@@ -34,7 +32,7 @@ class Puppet::Parser::AST
 
             pnames = nil
             if pklass = self.parentobj
-                pklass.safeevaluate :scope => scope, :resource => options[:resource]
+                pklass.safeevaluate(scope, resource)
 
                 scope = parent_scope(scope, pklass)
                 pnames = scope.namespaces
@@ -42,8 +40,8 @@ class Puppet::Parser::AST
 
             # Don't create a subscope for the top-level class, since it already
             # has its own scope.
-            unless options[:resource].title == :main
-                scope = subscope(scope, options[:resource])
+            unless resource.title == :main
+                scope = subscope(scope, resource)
             end
 
             if pnames
@@ -58,7 +56,7 @@ class Puppet::Parser::AST
 
             # Now evaluate our code, yo.
             if self.code
-                return self.code.evaluate(:scope => scope)
+                return self.code.safeevaluate(scope)
             else
                 return nil
             end
diff --git a/lib/puppet/parser/ast/ifstatement.rb b/lib/puppet/parser/ast/ifstatement.rb
index 66a07b01f..afa2cd572 100644
--- a/lib/puppet/parser/ast/ifstatement.rb
+++ b/lib/puppet/parser/ast/ifstatement.rb
@@ -12,15 +12,14 @@ class Puppet::Parser::AST
         # Short-curcuit evaluation.  If we're true, evaluate our statements,
         # else if there's an 'else' setting, evaluate it.
         # the first option that matches.
-        def evaluate(hash)
-            scope = hash[:scope]
-            value = @test.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            value = @test.safeevaluate(scope)
 
             if Puppet::Parser::Scope.true?(value)
-                return @statements.safeevaluate(:scope => scope)
+                return @statements.safeevaluate(scope)
             else
                 if defined? @else
-                    return @else.safeevaluate(:scope => scope)
+                    return @else.safeevaluate(scope)
                 else
                     return nil
                 end
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 225253061..c545c1e47 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -6,7 +6,7 @@ class Puppet::Parser::AST
         attr_accessor :value, :type
 
         # Return our value.
-        def evaluate(hash)
+        def evaluate(scope)
             return @value
         end
 
@@ -35,14 +35,14 @@ class Puppet::Parser::AST
     class String < AST::Leaf
         # Interpolate the string looking for variables, and then return
         # the result.
-        def evaluate(hash)
-            return hash[:scope].strinterp(@value, @file, @line)
+        def evaluate(scope)
+            return scope.strinterp(@value, @file, @line)
         end
     end
 
     # An uninterpreted string.
     class FlatString < AST::Leaf
-        def evaluate(hash)
+        def evaluate(scope)
             return @value
         end
     end
@@ -81,9 +81,9 @@ class Puppet::Parser::AST
     class Variable < Name
         # Looks up the value of the object in the scope tree (does
         # not include syntactical constructs, like '$' and '{}').
-        def evaluate(hash)
+        def evaluate(scope)
             parsewrap do
-                return hash[:scope].lookupvar(@value)
+                return scope.lookupvar(@value)
             end
         end
     end
diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb
index a296e43ba..3c5d44d1b 100644
--- a/lib/puppet/parser/ast/node.rb
+++ b/lib/puppet/parser/ast/node.rb
@@ -7,23 +7,15 @@ class Puppet::Parser::AST
         @name = :node
         attr_accessor :name
 
-        def evaluate(options)
-            scope = options[:scope]
-
-            #pscope = if ! Puppet[:lexical] or options[:asparent]
-            #    @scope
-            #else
-            #    origscope
-            #end
-
+        def evaluate(scope, resource)
             # We don't have to worry about the declarativeness of node parentage,
             # because the entry point is always a single node definition.
             if parent = self.parentobj
-                scope = parent.safeevaluate :scope => scope, :resource => options[:resource]
+                scope = parent.safeevaluate scope, resource
             end
 
             scope = scope.newscope(
-                :resource => options[:resource],
+                :resource => resource,
                 :keyword => @keyword,
                 :source => self,
                 :namespace => "" # nodes are always in ""
@@ -36,7 +28,7 @@ class Puppet::Parser::AST
 
             # And then evaluate our code if we have any
             if self.code
-                @code.safeevaluate(:scope => scope)
+                @code.safeevaluate(scope)
             end
 
             return scope
diff --git a/lib/puppet/parser/ast/resource.rb b/lib/puppet/parser/ast/resource.rb
index c53ab0a68..606beb537 100644
--- a/lib/puppet/parser/ast/resource.rb
+++ b/lib/puppet/parser/ast/resource.rb
@@ -9,15 +9,13 @@ class Resource < AST::ResourceReference
 
     # Does not actually return an object; instead sets an object
     # in the current scope.
-    def evaluate(options)
-        scope = options[:scope]
-
+    def evaluate(scope)
         # Evaluate all of the specified params.
         paramobjects = @params.collect { |param|
-            param.safeevaluate(:scope => scope)
+            param.safeevaluate(scope)
         }
 
-        objtitles = @title.safeevaluate(:scope => scope)
+        objtitles = @title.safeevaluate(scope)
 
         # it's easier to always use an array, even for only one name
         unless objtitles.is_a?(Array)
diff --git a/lib/puppet/parser/ast/resource_defaults.rb b/lib/puppet/parser/ast/resource_defaults.rb
index 8f9c1b8df..4856f0594 100644
--- a/lib/puppet/parser/ast/resource_defaults.rb
+++ b/lib/puppet/parser/ast/resource_defaults.rb
@@ -8,13 +8,11 @@ class Puppet::Parser::AST
 
         # As opposed to ResourceDef, this stores each default for the given
         # object type.
-        def evaluate(hash)
-            scope = hash[:scope]
-
+        def evaluate(scope)
             # Use a resource reference to canonize the type
             ref = Puppet::ResourceReference.new(@type, "whatever")
             type = ref.type
-            params = @params.safeevaluate(:scope => scope)
+            params = @params.safeevaluate(scope)
 
             parsewrap do
                 scope.setdefaults(type, params)
diff --git a/lib/puppet/parser/ast/resource_override.rb b/lib/puppet/parser/ast/resource_override.rb
index 46c930902..d15f68608 100644
--- a/lib/puppet/parser/ast/resource_override.rb
+++ b/lib/puppet/parser/ast/resource_override.rb
@@ -17,17 +17,15 @@ class Puppet::Parser::AST
 
         # Does not actually return an object; instead sets an object
         # in the current scope.
-        def evaluate(hash)
-            scope = hash[:scope]
-
+        def evaluate(scope)
             # Get our object reference.
-            object = @object.safeevaluate(:scope => scope)
+            object = @object.safeevaluate(scope)
 
             hash = {}
 
             # Evaluate all of the specified params.
             params = @params.collect { |param|
-                param.safeevaluate(:scope => scope)
+                param.safeevaluate(scope)
             }
 
             # Now we just create a normal resource, but we call a very different
diff --git a/lib/puppet/parser/ast/resource_reference.rb b/lib/puppet/parser/ast/resource_reference.rb
index b06ea17be..4bb41165a 100644
--- a/lib/puppet/parser/ast/resource_reference.rb
+++ b/lib/puppet/parser/ast/resource_reference.rb
@@ -22,10 +22,8 @@ class Puppet::Parser::AST
 
         # Evaluate our object, but just return a simple array of the type
         # and name.
-        def evaluate(hash)
-            scope = hash[:scope]
-
-            title = @title.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            title = @title.safeevaluate(scope)
             if @type.to_s.downcase == "class"
                 objtype = "class"
                 title = qualified_class(scope, title)
diff --git a/lib/puppet/parser/ast/resourceparam.rb b/lib/puppet/parser/ast/resourceparam.rb
index 8b1e7b367..c552a7ee5 100644
--- a/lib/puppet/parser/ast/resourceparam.rb
+++ b/lib/puppet/parser/ast/resourceparam.rb
@@ -10,12 +10,10 @@ class Puppet::Parser::AST
         end
 
         # Return the parameter and the value.
-        def evaluate(hash)
-            scope = hash[:scope]
-
+        def evaluate(scope)
             return Puppet::Parser::Resource::Param.new(
                 :name => @param,
-                :value => @value.safeevaluate(:scope => scope),
+                :value => @value.safeevaluate(scope),
                 :source => scope.source, :line => self.line, :file => self.file,
                 :add => self.add
             )
diff --git a/lib/puppet/parser/ast/selector.rb b/lib/puppet/parser/ast/selector.rb
index d363ab7e4..399d405a3 100644
--- a/lib/puppet/parser/ast/selector.rb
+++ b/lib/puppet/parser/ast/selector.rb
@@ -11,13 +11,12 @@ class Puppet::Parser::AST
         end
 
         # Find the value that corresponds with the test.
-        def evaluate(hash)
-            scope = hash[:scope]
+        def evaluate(scope)
             retvalue = nil
             found = nil
 
             # Get our parameter.
-            paramvalue = @param.safeevaluate(:scope => scope)
+            paramvalue = @param.safeevaluate(scope)
             
             sensitive = Puppet[:casesensitive]
             
@@ -33,13 +32,13 @@ class Puppet::Parser::AST
 
             # Then look for a match in the options.
             @values.each { |obj|
-                param = obj.param.safeevaluate(:scope => scope)
+                param = obj.param.safeevaluate(scope)
                 if ! sensitive && param.respond_to?(:downcase)
                     param = param.downcase
                 end
                 if param == paramvalue
                     # we found a matching option
-                    retvalue = obj.value.safeevaluate(:scope => scope)
+                    retvalue = obj.value.safeevaluate(scope)
                     found = true
                     break
                 elsif obj.param.is_a?(Default)
@@ -51,7 +50,7 @@ class Puppet::Parser::AST
             # Unless we found something, look for the default.
             unless found
                 if default
-                    retvalue = default.value.safeevaluate(:scope => scope)
+                    retvalue = default.value.safeevaluate(scope)
                 else
                     self.fail Puppet::ParseError,
                         "No matching value for selector param '%s'" % paramvalue
diff --git a/lib/puppet/parser/ast/tag.rb b/lib/puppet/parser/ast/tag.rb
index e2882d2f0..2909504a7 100644
--- a/lib/puppet/parser/ast/tag.rb
+++ b/lib/puppet/parser/ast/tag.rb
@@ -8,10 +8,8 @@ class Puppet::Parser::AST
         @name = :class
         attr_accessor :type
 
-        def evaluate(hash)
-            scope = hash[:scope]
-
-            types = @type.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            types = @type.safeevaluate(scope)
 
             types = [types] unless types.is_a? Array
 
diff --git a/lib/puppet/parser/ast/vardef.rb b/lib/puppet/parser/ast/vardef.rb
index 1e7f874bc..ee79159d7 100644
--- a/lib/puppet/parser/ast/vardef.rb
+++ b/lib/puppet/parser/ast/vardef.rb
@@ -9,10 +9,9 @@ class Puppet::Parser::AST
 
         # Look up our name and value, and store them appropriately.  The
         # lexer strips off the syntax stuff like '$'.
-        def evaluate(hash)
-            scope = hash[:scope]
-            name = @name.safeevaluate(:scope => scope)
-            value = @value.safeevaluate(:scope => scope)
+        def evaluate(scope)
+            name = @name.safeevaluate(scope)
+            value = @value.safeevaluate(scope)
 
             parsewrap do
                 scope.setvar(name,value, @file, @line)
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 58667727f..ea6038299 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -59,7 +59,7 @@ class Puppet::Parser::Resource
         if klass = @ref.definedtype
             finish()
             scope.compile.delete_resource(self)
-            return klass.evaluate(:scope => scope, :resource => self)
+            return klass.evaluate(scope, self)
         elsif builtin?
             devfail "Cannot evaluate a builtin type"
         else