The language now verifies some resemblance to closurehood. I now only need to fix the library to expect this behaviour.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@736 980ebf18-57e1-0310-9a29-db15c13687c0

3 files changed, 147 insertions, 48 deletions

diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index fa21f8a9e..dd56c109a 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -1228,7 +1228,8 @@ module Puppet
                     end
                     
                     names.each { |name|
-                        Puppet.debug("defining host '%s'" % name)
+                        Puppet.debug("defining host '%s' in scope %s" %
+                            [name, scope.object_id])
                         arghash = {
                             :name => name,
                             :code => @code
@@ -1285,9 +1286,10 @@ module Puppet
                 # The class name
                 @name = :component
 
-                attr_accessor :name, :args, :code
+                attr_accessor :name, :args, :code, :scope
 
                 def evaluate(scope,hash,objtype,objname)
+
                     scope = scope.newscope
 
                     # The type is the component or class name
@@ -1297,9 +1299,18 @@ module Puppet
                     # been dynamically generated.  This is almost never used
                     scope.name = objname
 
+                    #if self.is_a?(Node)
+                    #    scope.isnodescope
+                    #end
+
                     # Additionally, add a tag for whatever kind of class
                     # we are
-                    scope.base = self.class.name
+                    scope.tag(objtype)
+
+                    unless objname =~ /-\d+/ # it was generated
+                        scope.tag(objname)
+                    end
+                    #scope.base = self.class.name
 
 
                     # define all of the arguments in our local scope
@@ -1354,6 +1365,10 @@ module Puppet
 
                     # Now just evaluate the code with our new bindings.
                     self.code.safeevaluate(scope)
+
+                    # We return the scope, so that our children can make their scopes
+                    # under ours.  This allows them to find our definitions.
+                    return scope
                 end
             end
 
@@ -1370,7 +1385,13 @@ module Puppet
                         return nil
                     end
 
-                    self.evalparent(scope, hash, objname)
+                    if tmp = self.evalparent(scope, hash, objname)
+                        # Override our scope binding with the parent scope
+                        # binding. This is quite hackish, but I can't think
+                        # of another way to make sure our scopes end up under
+                        # our parent scopes.
+                        scope = tmp
+                    end
 
                     # just use the Component evaluate method, but change the type
                     # to our own type
@@ -1382,7 +1403,9 @@ module Puppet
                     return retval
                 end
 
-                # Evaluate our parent class.  
+                # Evaluate our parent class.  Parent classes are evaluated in the
+                # exact same scope as the children.  This is maybe not a good idea
+                # but, eh.
                 def evalparent(scope, args, name)
                     if @parentclass
                         parentobj = nil
@@ -1411,14 +1434,14 @@ module Puppet
                         # Verify that the parent and child are of the same type
                         unless parentobj.class == self.class
                             error = Puppet::ParseError.new(
-                                "Class %s has incompatible parent type" %
-                                [@name]
+                                "Class %s has incompatible parent type, %s vs %s" %
+                                [@name, parentobj.class, self.class]
                             )
                             error.file = self.file
                             error.line = self.line
                             raise error
                         end
-                        parentobj.safeevaluate(scope,args,@parentclass,name)
+                        return parentobj.safeevaluate(scope,args,@parentclass,name)
                     end
                 end
 
@@ -1429,8 +1452,9 @@ module Puppet
 
             end
 
-            # The specific code associated with a host.  
-            class Node < AST::Component
+            # The specific code associated with a host.  Nodes are annoyingly unlike
+            # other objects.  That's just the way it is, at least for now.
+            class Node < AST::HostClass
                 @name = :node
                 attr_accessor :name, :args, :code, :parentclass
 
@@ -1448,14 +1472,24 @@ module Puppet
 
                     # Mark this scope as a nodescope, so that classes will be
                     # singletons within it
-                    scope.nodescope = true
+                    scope.isnodescope
 
                     # Now set all of the facts inside this scope
                     facts.each { |var, value|
                         scope.setvar(var, value)
                     }
 
-                    self.evalparent(scope)
+                    if tmp = self.evalparent(scope)
+                        # Again, override our scope with the parent scope, if
+                        # there is one.
+                        scope = tmp
+                    end
+
+                    #scope.tag(@name)
+
+                    # We never pass the facts to the parent class, because they've
+                    # already been defined at this top-level scope.
+                    #super(scope, facts, @name, @name)
 
                     # And then evaluate our code.
                     @code.safeevaluate(scope)
@@ -1489,7 +1523,7 @@ module Puppet
                         node = hash[:node]
                         # Tag the scope with the parent's name/type.
                         name = node.name
-                        Puppet.info "Tagging with parent node %s" % name
+                        #Puppet.info "Tagging with parent node %s" % name
                         scope.tag(name)
 
                         begin
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 89e6ab75d..6e0854206 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -11,8 +11,8 @@ module Puppet
             attr_accessor :name, :type, :topscope, :base
 
             # This is probably not all that good of an idea, but...
-            # This way a parent can share its node table with all of its children.
-            attr_writer :nodetable 
+            # This way a parent can share its tables with all of its children.
+            attr_writer :nodetable, :classtable, :definedtable
 
             # Whether we behave declaratively.  Note that it's a class variable,
             # so all scopes behave the same.
@@ -70,6 +70,18 @@ module Puppet
                 else
                     raise Puppet::DevError, "No nodetable has been defined"
                 end
+
+                if defined? @classtable
+                    scope.classtable = @classtable
+                else
+                    raise Puppet::DevError, "No classtable has been defined"
+                end
+
+                if defined? @definedtable
+                    scope.definedtable = @definedtable
+                else
+                    raise Puppet::DevError, "No definedtable has been defined"
+                end
             end
 
             # Test whether a given scope is declarative.  Even though it's
@@ -108,8 +120,28 @@ module Puppet
                 @nodescope
             end
 
-            def nodescope=(bool)
-                @nodescope = bool
+            # Mark that we are a nodescope.
+            def isnodescope
+                @nodescope = true
+
+                # Also, create the extra tables associated with being a node
+                # scope.
+                # The table for storing class singletons.
+                @classtable = Hash.new(nil)
+
+                # Also, create the object checking map
+                @definedtable = Hash.new { |types, type|
+                    types[type] = {}
+                }
+            end
+
+            # Has a given object been defined anywhere in the scope tree?
+            def objectdefined?(name, type)
+                unless defined? @definedtable
+                    raise Puppet::DevError, "Scope did not receive definedtable"
+                end
+
+                return @definedtable[type][name]
             end
 
             # Are we the top scope?
@@ -197,7 +229,10 @@ module Puppet
                 end
             end
 
-            # Evaluate normally, with no node definitions
+            # Evaluate normally, with no node definitions.  This is a bit of a
+            # silly method, in that it just calls evaluate on the passed-in
+            # objects, and then calls to_trans on itself.  It just conceals
+            # a paltry amount of info from whomever's using the scope object.
             def evaluate(objects, facts = {})
                 facts.each { |var, value|
                     self.setvar(var, value)
@@ -222,6 +257,16 @@ module Puppet
 
                     @@declarative = declarative
 
+                    # The table for storing class singletons.  This will only actually
+                    # be used by top scopes and node scopes.
+                    @classtable = Hash.new(nil)
+
+                    # The table for all defined objects.  This will only be
+                    # used in the top scope if we don't have any nodescopes.
+                    @definedtable = Hash.new { |types, type|
+                        types[type] = {}
+                    }
+
                     # A table for storing nodes.
                     @nodetable = Hash.new(nil)
 
@@ -256,10 +301,6 @@ module Puppet
                 # The type table for this scope
                 @typetable = Hash.new(nil)
 
-                # The table for storing class singletons.  This will only actually
-                # be used by top scopes and node scopes.
-                @classtable = Hash.new(nil)
-
                 # All of the defaults set for types.  It's a hash of hashes,
                 # with the first key being the type, then the second key being
                 # the parameter.
@@ -321,14 +362,10 @@ module Puppet
             # Look up a given class.  This enables us to make sure classes are
             # singletons
             def lookupclass(klass)
-                if self.nodescope? or self.topscope?
-                    return @classtable[klass]
-                else
-                    unless @parent
-                        raise Puppet::DevError, "Not top scope but not parent defined"
-                    end
-                    return @parent.lookupclass(klass)
+                unless defined? @classtable
+                    raise Puppet::DevError, "Scope did not receive class table"
                 end
+                return @classtable[klass]
             end
 
             # Collect all of the defaults set at any higher scopes.
@@ -492,14 +529,34 @@ module Puppet
                 return newstring
             end
 
-            # This is kind of quirky, because it doesn't differentiate between
-            # creating a new object and adding params to an existing object.
-            # It doesn't solve the real problem, though: cases like file recursion,
-            # where one statement explicitly modifies an object, and another
-            # statement modifies it because of recursion.
+            # This method will fail if the named object is already defined anywhere
+            # in the scope tree, which is what provides some minimal closure-like
+            # behaviour.
             def setobject(type, name, params, file, line)
+                # First see if we can look the object up using normal scope
+                # rules, i.e., one of our parent classes has defined the
+                # object or something
                 obj = self.lookupobject(name,type)
+
+                # If we can't find it...
                 if obj == :undefined or obj.nil?
+                    # Make sure it's not defined elsewhere in the configuration
+                    if tmp = self.objectdefined?(name, type)
+                        msg = "Duplicate definition: %s[%s] is already defined" %
+                            [type, name]
+                        if tmp.line
+                            msg += " at line %s" % tmp.line
+                        end
+                        if tmp.file
+                            msg += " in file %s" % tmp.file
+                        end
+                        error = Puppet::ParseError.new(msg)
+                        error.file = file
+                        error.line = line
+                        raise error
+                    end
+
+                    # And if it's not, then create it anew
                     obj = @objectable[type][name]
 
                     # only set these if we've created the object, which is the
@@ -508,13 +565,16 @@ module Puppet
                     obj.line = line
                 end
 
-                # now add the params to whatever object we've found, whether
-                # it was in a higher scope or we just created it
-                # it will not be obvious where these parameters are from, that is,
-                # which file they're in or whatever
+                # Now add our parameters.  This has the function of overriding
+                # existing values, which might have been defined in a higher
+                # scope.
                 params.each { |var,value|
                     obj[var] = value
                 }
+
+                # And finally store the fact that we've defined this object.
+                @definedtable[type][name] = obj
+
                 return obj
             end
 
@@ -538,14 +598,15 @@ module Puppet
                 end
             end
 
-            # Add a tag to our current list.  This is only currently used
-            # when nodes evaluate their parents.
+            # Add a tag to our current list.  These tags will be added to all
+            # of the objects contained in this scope.
             def tag(*ary)
                 ary.each { |tag|
                     if tag.nil? or tag == ""
                         Puppet.warning "got told to tag with %s" % tag.inspect
                     end
                     unless @tags.include?(tag)
+                        #Puppet.info "Tagging scope %s with %s" % [self.object_id, tag]
                         @tags << tag.to_s
                     end
                 }
@@ -593,8 +654,10 @@ module Puppet
                                 results.push(result)
                             }
                         else
-                            # Otherwise, just add it to our list of results.
-                            results.push(cresult)
+                            unless cresult.empty?
+                                # Otherwise, just add it to our list of results.
+                                results.push(cresult)
+                            end
                         end
 
                         # Nodescopes are one-time; once they've been evaluated
@@ -604,6 +667,9 @@ module Puppet
                             @children.delete(child)
                         end
                     elsif child.is_a?(TransObject)
+                        if child.empty?
+                            next
+                        end
                         # Wait until the last minute to set tags, although this
                         # probably should not matter
                         child.tags = self.tags
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index a9dea65ec..5ea00713e 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -1,11 +1,8 @@
-#!/usr/local/bin/ruby -w
-
-# $Id$
-
 require 'puppet'
 
 module Puppet
-    #------------------------------------------------------------
+    # The transportable objects themselves.  Basically just a hash with some
+    # metadata and a few extra methods.
     class TransObject < Hash
         attr_accessor :type, :name, :file, :line
 
@@ -23,7 +20,7 @@ module Puppet
         end
 
         def tags
-            return @tags + [@type, @name]
+            return @tags
         end
 
         def to_s
@@ -167,3 +164,5 @@ module Puppet
     end
     #------------------------------------------------------------
 end
+
+# $Id$