Move scope parenting & class_scope from Compiler to Scope

This refactor fixes about a quarter of the test failures on master and (I
hope) will simplify some of the integration issues on the testing branch.
It is my best guess at The Right Thing To Do (or at least a step in that
direction) but I could be persuaded otherwise.

The basic idea is to take responsibility for maintaining scope hierarchy and
class_name -> class_scope mapping out of the compiler class and put it in the
scope class where it arguably belongs.  To maintain the semantics, class
scopes are all tracked by the "top level" scope, though this could be relaxed
if the nesting semantics were ever needed.

If this winds up being the right thing to do, related routines (e.g. newscope)
should be sorted out as well.

3 files changed, 37 insertions, 57 deletions

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 6b6cd6815..51df86ec6 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -51,32 +51,11 @@ class Puppet::Parser::Compiler
         parser.nodes?
     end
 
-    # Store the fact that we've evaluated a class, and store a reference to
-    # the scope in which it was evaluated, so that we can look it up later.
-    def class_set(name, scope)
-        if existing = @class_scopes[name]
-            if existing.nodescope? != scope.nodescope?
-                raise Puppet::ParseError, "Cannot have classes, nodes, or definitions with the same name"
-            else
-                raise Puppet::DevError, "Somehow evaluated %s %s twice" % [ existing.nodescope? ? "node" : "class", name]
-            end
-        end
-        @class_scopes[name] = scope
+    # Store the fact that we've evaluated a class
+    def add_class(name)
         @catalog.add_class(name) unless name == ""
     end
 
-    # Return the scope associated with a class.  This is just here so
-    # that subclasses can set their parent scopes to be the scope of
-    # their parent class, and it's also used when looking up qualified
-    # variables.
-    def class_scope(klass)
-        # They might pass in either the class or class name
-        if klass.respond_to?(:name)
-            @class_scopes[klass.name]
-        else
-            @class_scopes[klass]
-        end
-    end
 
     # Return a list of all of the defined classes.
     def classlist
@@ -138,7 +117,7 @@ class Puppet::Parser::Compiler
         classes.each do |name|
             # If we can find the class, then make a resource that will evaluate it.
             if klass = scope.find_hostclass(name)
-                found << name and next if class_scope(klass)
+                found << name and next if scope.class_scope(klass)
 
                 resource = klass.mk_plain_resource(scope)
 
@@ -180,27 +159,16 @@ class Puppet::Parser::Compiler
     end
 
     # Create a new scope, with either a specified parent scope or
-    # using the top scope.  Adds an edge between the scope and
-    # its parent to the graph.
+    # using the top scope.  
     def newscope(parent, options = {})
         parent ||= topscope
         options[:compiler] = self
         options[:parser] ||= self.parser
         scope = Puppet::Parser::Scope.new(options)
-        @scope_graph.add_edge(parent, scope)
+        scope.parent = parent
         scope
     end
 
-    # Find the parent of a given scope.  Assumes scopes only ever have
-    # one in edge, which will always be true.
-    def parent(scope)
-        if ary = @scope_graph.adjacent(scope, :direction => :in) and ary.length > 0
-            ary[0]
-        else
-            nil
-        end
-    end
-
     # Return any overrides for the given resource.
     def resource_overrides(resource)
         @resource_overrides[resource.ref]
@@ -236,7 +204,7 @@ class Puppet::Parser::Compiler
 
         # Now set the node scope appropriately, so that :topscope can
         # behave differently.
-        @node_scope = class_scope(astnode)
+        @node_scope = topscope.class_scope(astnode)
     end
 
     # Evaluate our collections and return true if anything returned an object.
@@ -384,15 +352,10 @@ class Puppet::Parser::Compiler
     def init_main
         # Create our initial scope and a resource that will evaluate main.
         @topscope = Puppet::Parser::Scope.new(:compiler => self, :parser => self.parser)
-        @scope_graph.add_vertex(@topscope)
     end
 
     # Set up all of our internal variables.
     def initvars
-        # The table for storing class singletons.  This will only actually
-        # be used by top scopes and node scopes.
-        @class_scopes = {}
-
         # The list of objects that will available for export.
         @exported_resources = {}
 
@@ -406,9 +369,6 @@ class Puppet::Parser::Compiler
         # but they each refer back to the scope that created them.
         @collections = []
 
-        # A graph for maintaining scope relationships.
-        @scope_graph = Puppet::SimpleGraph.new
-
         # For maintaining the relationship between scopes and their resources.
         @catalog = Puppet::Resource::Catalog.new(@node.name)
         @catalog.version = @parser.version
diff --git a/lib/puppet/parser/resource_type.rb b/lib/puppet/parser/resource_type.rb
index a67965960..3dbcbcb62 100644
--- a/lib/puppet/parser/resource_type.rb
+++ b/lib/puppet/parser/resource_type.rb
@@ -165,6 +165,7 @@ class Puppet::Parser::ResourceType
 
         scope.setvar("title", resource.title) unless set.include? :title
         scope.setvar("name", resource.name) unless set.include? :name
+        scope.class_set(self.name,scope)
     end
 
     # Create a new subscope in which to evaluate our code.
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index d6d663041..eda53a8cb 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -18,7 +18,7 @@ class Puppet::Parser::Scope
     attr_accessor :level, :parser, :source, :resource
     attr_accessor :base, :keyword, :nodescope
     attr_accessor :top, :translated, :compiler
-    attr_writer :parent
+    attr_accessor :parent
 
     # A demeterific shortcut to the catalog.
     def catalog
@@ -139,6 +139,34 @@ class Puppet::Parser::Scope
         @defaults = Hash.new { |dhash,type|
             dhash[type] = {}
         }
+
+        # The table for storing class singletons.  This will only actually
+        # be used by top scopes and node scopes.
+        @class_scopes = {}
+    end
+
+    # Store the fact that we've evaluated a class, and store a reference to
+    # the scope in which it was evaluated, so that we can look it up later.
+    def class_set(name, scope)
+        return parent.class_set(name,scope) if parent
+        if existing = @class_scopes[name]
+            if existing.nodescope? != scope.nodescope?
+                raise Puppet::ParseError, "Cannot have classes, nodes, or definitions with the same name"
+            else
+                raise Puppet::DevError, "Somehow evaluated %s %s twice" % [ existing.nodescope? ? "node" : "class", name]
+            end
+        end
+        @class_scopes[name] = scope
+    end
+
+    # Return the scope associated with a class.  This is just here so
+    # that subclasses can set their parent scopes to be the scope of
+    # their parent class, and it's also used when looking up qualified
+    # variables.
+    def class_scope(klass)
+        # They might pass in either the class or class name
+        k = klass.respond_to?(:name) ? klass.name : klass
+        @class_scopes[k] || (parent && parent.class_scope(k))
     end
 
     # Collect all of the defaults set at any higher scopes.
@@ -182,7 +210,7 @@ class Puppet::Parser::Scope
             warning "Could not look up qualified variable '%s'; class %s could not be found" % [name, klassname]
             return usestring ? "" : :undefined
         end
-        unless kscope = compiler.class_scope(klass)
+        unless kscope = class_scope(klass)
             warning "Could not look up qualified variable '%s'; class %s has not been evaluated" % [name, klassname]
             return usestring ? "" : :undefined
         end
@@ -251,15 +279,6 @@ class Puppet::Parser::Scope
         self.nodescope
     end
 
-    # We probably shouldn't cache this value...  But it's a lot faster
-    # than doing lots of queries.
-    def parent
-        unless defined?(@parent)
-            @parent = compiler.parent(self)
-        end
-        @parent
-    end
-
     # Return the list of scopes up to the top scope, ordered with our own first.
     # This is used for looking up variables and defaults.
     def scope_path