Adding some small changes towards fixing #140 and #83, but this work needs to take a back seat to object collection, so i will come back to it later.

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

8 files changed, 146 insertions, 30 deletions

diff --git a/lib/puppet/parser/ast/compdef.rb b/lib/puppet/parser/ast/compdef.rb
index f1b947ec4..e5758d6f0 100644
--- a/lib/puppet/parser/ast/compdef.rb
+++ b/lib/puppet/parser/ast/compdef.rb
@@ -9,7 +9,7 @@ class Puppet::Parser::AST
     # encounter an error if the component is instantiated more than
     # once.
     class CompDef < AST::Branch
-        attr_accessor :type, :args, :code, :keyword
+        attr_accessor :type, :args, :code, :keyword, :scope
 
         def each
             [@type,@args,@code].each { |child| yield child }
diff --git a/lib/puppet/parser/ast/component.rb b/lib/puppet/parser/ast/component.rb
index 50e8df5a8..5cf3f5c57 100644
--- a/lib/puppet/parser/ast/component.rb
+++ b/lib/puppet/parser/ast/component.rb
@@ -19,7 +19,13 @@ class Puppet::Parser::AST
             objname = hash[:name]
             arguments = hash[:arguments] || {}
 
-            scope = origscope.newscope(
+            pscope = origscope
+            #pscope = if ! Puppet[:lexical] or hash[:asparent] == false
+            #    origscope
+            #else
+            #    @scope
+            #end
+            scope = pscope.newscope(
                 :type => @type,
                 :name => objname,
                 :keyword => self.keyword
@@ -32,17 +38,13 @@ class Puppet::Parser::AST
             end
             @scope = scope
 
-
             # Additionally, add a tag for whatever kind of class
             # we are
             scope.tag(@type)
 
             unless objname.nil?
-                #Puppet.info "tagging with %s" % objname.inspect
                 scope.tag(objname)
             end
-            #scope.base = self.class.name
-
 
             # define all of the arguments in our local scope
             if self.args
diff --git a/lib/puppet/parser/ast/hostclass.rb b/lib/puppet/parser/ast/hostclass.rb
index 85beecfe7..7f381db2a 100644
--- a/lib/puppet/parser/ast/hostclass.rb
+++ b/lib/puppet/parser/ast/hostclass.rb
@@ -14,8 +14,15 @@ class Puppet::Parser::AST
             # Verify that we haven't already been evaluated
             # FIXME The second subclass won't evaluate the parent class
             # code at all, and any overrides will throw an error.
-            if scope.lookupclass(self.object_id)
+            if myscope = scope.lookupclass(self.object_id)
                 Puppet.debug "%s class already evaluated" % @type
+
+                # Not used, but will eventually be used to fix #140.
+                if myscope.is_a? Puppet::Parser::Scope
+                    unless scope.object_id == myscope.object_id
+                        #scope.parent = myscope
+                    end
+                end
                 return nil
             end
 
@@ -55,9 +62,14 @@ class Puppet::Parser::AST
                 :type => @type,
                 :name => objname,               # might be nil
                 :newcontext => newcontext,
-                :asparent => hash[:asparent]    # might be nil
+                :asparent => hash[:asparent] || false    # might be nil
             )
 
+            # Now set the class again, this time using the scope.  This way
+            # we can look up the parent scope of this class later, so we
+            # can hook the children together.
+            scope.setscope(self.object_id, result)
+
             # This is important but painfully difficult.  If we're the top-level
             # class, that is, we have no parent classes, then the transscope
             # is our own scope, but if there are parent classes, then the topmost
diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb
index f1b128ccf..e4e69bed9 100644
--- a/lib/puppet/parser/ast/node.rb
+++ b/lib/puppet/parser/ast/node.rb
@@ -7,13 +7,21 @@ class Puppet::Parser::AST
 
         #def evaluate(scope, facts = {})
         def evaluate(hash)
-            scope = hash[:scope]
+            origscope = hash[:scope]
             facts = hash[:facts] || {}
-            #scope.info "name is %s, type is %s" % [self.name, self.type]
+
             # nodes are never instantiated like a normal object,
             # but we need the type to be the name users would use for
             # instantiation, otherwise tags don't work out
-            scope = scope.newscope(
+
+            pscope = origscope
+            #pscope = if ! Puppet[:lexical] or hash[:asparent]
+            #    @scope
+            #else
+            #    origscope
+            #end
+
+            scope = pscope.newscope(
                 :type => self.type,
                 :keyword => @keyword
             )
@@ -78,7 +86,7 @@ class Puppet::Parser::AST
 
                 begin
                     code = node.code
-                    code.safeevaluate(:scope => scope)
+                    code.safeevaluate(:scope => scope, :asparent => true)
                 rescue Puppet::ParseError => except
                     except.line = self.line
                     except.file = self.file
diff --git a/lib/puppet/parser/ast/nodedef.rb b/lib/puppet/parser/ast/nodedef.rb
index cc710329f..06b104828 100644
--- a/lib/puppet/parser/ast/nodedef.rb
+++ b/lib/puppet/parser/ast/nodedef.rb
@@ -3,7 +3,7 @@ class Puppet::Parser::AST
     # specified node, and this parse tree is only ever looked up when
     # a client connects.
     class NodeDef < AST::Branch
-        attr_accessor :names, :code, :parentclass, :keyword
+        attr_accessor :names, :code, :parentclass, :keyword, :scope
 
         def each
             [@names,@code].each { |child| yield child }
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 03370558d..f7dde32c8 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -10,13 +10,17 @@ module Puppet::Parser
             attr_accessor :file, :line, :type, :name
         end
 
+        # This doesn't actually work right now.
+        Puppet.config.setdefaults(:puppet,
+            :lexical => [false, "Whether to use lexical scoping (vs. dynamic)."])
+
         Puppet::Util.logmethods(self)
 
         include Enumerable
         attr_accessor :parent, :level, :interp
         attr_accessor :name, :type, :topscope, :base, :keyword
 
-        attr_accessor :top, :context
+        attr_accessor :top, :context, :translated
 
         # This is probably not all that good of an idea, but...
         # This way a parent can share its tables with all of its children.
@@ -174,7 +178,15 @@ module Puppet::Parser
             unless defined? @classtable
                 raise Puppet::DevError, "Scope did not receive class table"
             end
-            return @classtable.values
+            return @classtable.collect { |id, klass|
+                # The class table can contain scopes or strings as its values
+                # so support them accordingly.
+                if klass.is_a? Scope
+                    klass.type
+                else
+                    klass
+                end
+            }
         end
 
         # Yield each child scope in turn
@@ -284,6 +296,11 @@ module Puppet::Parser
                 bucket.classes = self.classlist
             end
 
+            # Now clean up after ourselves
+            [@@done].each do |table|
+                table.clear
+            end
+
             return bucket
         end
 
@@ -577,11 +594,13 @@ module Puppet::Parser
                 raise Puppet::ParseError, "Invalid class name '%s'" % name
             end
 
-            if self.topscope?
-                @classtable[id] = name
-            else
-                @parent.setclass(id, name)
-            end
+            @classtable[id] = name
+        end
+
+        # Store the scope for each class, so that other subclasses can look
+        # them up.
+        def setscope(id, scope)
+            @classtable[id] = scope
         end
 
         # Set defaults for a type.  The typename should already be downcased,
@@ -629,6 +648,7 @@ module Puppet::Parser
                 # We have to store both the scope that's setting the node and
                 # the node itself, so that the node gets evaluated in the correct
                 # scope.
+                code.scope = self
                 @nodetable[name] = {
                     :scope => self,
                     :node => code
@@ -643,6 +663,7 @@ module Puppet::Parser
                 raise Puppet::ParseError,
                     "%s is already defined" % name
             else
+                ltype.scope = self
                 @typetable[name] = ltype
             end
         end
@@ -782,6 +803,7 @@ module Puppet::Parser
                 tmp << @type.to_s
             end
             if @parent
+                #info "Looking for tags in %s" % @parent.type
                 @parent.tags.each { |tag|
                     if tag.nil? or tag == ""
                         Puppet.debug "parent returned tag %s" % tag.inspect
@@ -809,6 +831,9 @@ module Puppet::Parser
         def to_trans
             results = []
 
+            # Set this on entry, just in case someone tries to get all weird
+            @translated = true
+
             @children.dup.each do |child|
                 if @@done.include?(child)
                     raise Puppet::DevError, "Already translated %s" % child.object_id
@@ -1033,12 +1058,15 @@ module Puppet::Parser
             elsif table.include?(sub)
                 return table[sub]
             elsif ! @parent.nil?
-                #self.notice "Context is %s, parent %s is %s" %
-                #    [self.context, @parent.type, @parent.context]
+                # Context is used for retricting overrides.
                 if usecontext and self.context != @parent.context
                     return :undefined
                 else
-                    return @parent.lookup(type,sub, usecontext)
+                    #if defined? @superscope and val = @superscope.lookup(type,sub, usecontext) and val != :undefined
+                    #    return val
+                    #else
+                        return @parent.lookup(type,sub, usecontext)
+                    #end
                 end
             else
                 return :undefined
diff --git a/test/language/ast.rb b/test/language/ast.rb
index 781e9ddd0..b714fcdea 100755
--- a/test/language/ast.rb
+++ b/test/language/ast.rb
@@ -752,6 +752,59 @@ class TestAST < Test::Unit::TestCase
                 "Could not find file %s" % file)
         end
     end
+
+    # To fix #140.  Currently non-functional.
+    def disabled_test_classreuse
+        children = []
+
+        # Create the parent class, with a definition in it.
+        children << classobj("parent", :code => AST::ASTArray.new(
+            :file => __FILE__,
+            :line => __LINE__,
+            :children => [
+                compobj("foo", :args => AST::ASTArray.new(
+                        :children => [nameobj("arg")]
+                    ),
+                    :code => AST::ASTArray.new(
+                        :file => __FILE__,
+                        :line => __LINE__,
+                        :children => [fileobj("/$arg")]
+                    )
+                ),
+                objectdef("foo", "ptest", {"arg" => "parentfoo"})
+            ]
+        ))
+
+        # Create child class, also trying to use that definition
+        children << classobj("child1", :parentclass => nameobj("parent"),
+            :code => AST::ASTArray.new(
+                :file => __FILE__,
+                :line => __LINE__,
+                :children => [
+                    objectdef("foo", "ctest", {"arg" => "childfoo"})
+                ]
+            )
+        )
+
+        # Call the parent first
+        children << functionobj("include", "parent")
+
+        # Then call the child, and make sure it can look up the definition
+        children << functionobj("include", "child1")
+
+        top = nil
+        assert_nothing_raised("Could not create top object") {
+            top = AST::ASTArray.new(
+                :children => children
+            )
+        }
+
+        objects = nil
+        assert_nothing_raised("Could not evaluate") {
+            scope = Puppet::Parser::Scope.new()
+            objects = scope.evaluate(:ast => top)
+        }
+    end
 end
 
 # $Id$
diff --git a/test/puppettest.rb b/test/puppettest.rb
index 375098d10..d0c7a5778 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -791,18 +791,31 @@ module ParserTesting
         }
     end
 
-    def fileobj(path, hash = {"owner" => "root"})
-        assert_nothing_raised("Could not create file %s" % path) {
+    def objectdef(type, name, params)
+        assert_nothing_raised("Could not create %s %s" % [type, name]) {
             return AST::ObjectDef.new(
-                :file => tempfile(),
-                :line => rand(100),
-                :name => stringobj(path),
-                :type => nameobj("file"),
-                :params => objectinst(hash)
+                :file => __FILE__,
+                :line => __LINE__,
+                :name => stringobj(name),
+                :type => nameobj(type),
+                :params => objectinst(params)
             )
         }
     end
 
+    def fileobj(path, hash = {"owner" => "root"})
+        assert_nothing_raised("Could not create file %s" % path) {
+            return objectdef("file", path, hash)
+#            return AST::ObjectDef.new(
+#                :file => tempfile(),
+#                :line => rand(100),
+#                :name => stringobj(path),
+#                :type => nameobj("file"),
+#                :params => objectinst(hash)
+#            )
+        }
+    end
+
     def nameobj(name)
         assert_nothing_raised("Could not create name %s" % name) {
             return AST::Name.new(