Moving the ast node tests to rspec (which I could have

*sworn* I did this weekend).  In the process, I fixed
a couple of bugs related to differentiating between
nodes and classes, and then cleaned up quite a few
error messages.

5 files changed, 24 insertions, 38 deletions

diff --git a/lib/puppet/parser/ast/definition.rb b/lib/puppet/parser/ast/definition.rb
index b4a90016a..2b7506446 100644
--- a/lib/puppet/parser/ast/definition.rb
+++ b/lib/puppet/parser/ast/definition.rb
@@ -25,9 +25,9 @@ class Puppet::Parser::AST::Definition < Puppet::Parser::AST::Branch
     # Create a resource that knows how to evaluate our actual code.
     def evaluate(scope)
         # Do nothing if the resource already exists; this provides the singleton nature classes need.
-        return if scope.catalog.resource(:class, self.classname)
+        return if scope.catalog.resource(self.class.name, self.classname)
 
-        resource = Puppet::Parser::Resource.new(:type => "class", :title => self.classname, :scope => scope, :source => scope.source)
+        resource = Puppet::Parser::Resource.new(:type => self.class.name, :title => self.classname, :scope => scope, :source => scope.source)
 
         scope.catalog.tag(*resource.tags)
 
diff --git a/lib/puppet/parser/ast/hostclass.rb b/lib/puppet/parser/ast/hostclass.rb
index f49016526..8d4d01660 100644
--- a/lib/puppet/parser/ast/hostclass.rb
+++ b/lib/puppet/parser/ast/hostclass.rb
@@ -20,7 +20,7 @@ class Puppet::Parser::AST::HostClass < Puppet::Parser::AST::Definition
 
     # Make sure our parent class has been evaluated, if we have one.
     def evaluate(scope)
-        if parentclass and ! scope.catalog.resource(:class, parentclass)
+        if parentclass and ! scope.catalog.resource(self.class.name, parentclass)
             resource = parentobj.evaluate(scope)
         end
 
@@ -39,7 +39,9 @@ class Puppet::Parser::AST::HostClass < Puppet::Parser::AST::Definition
 
         pnames = nil
         if pklass = self.parentobj
-            pklass.evaluate_code(resource)
+            parent_resource = resource.scope.compiler.catalog.resource(self.class.name, pklass.classname)
+            # This shouldn't evaluate if the class has already been evaluated.
+            pklass.evaluate_code(parent_resource)
 
             scope = parent_scope(scope, pklass)
             pnames = scope.namespaces
@@ -49,14 +51,15 @@ class Puppet::Parser::AST::HostClass < Puppet::Parser::AST::Definition
         # has its own scope.
         scope = subscope(scope, resource) unless resource.title == :main
 
+        # Add the parent scope namespaces to our own.
         if pnames
             pnames.each do |ns|
                 scope.add_namespace(ns)
             end
         end
 
-        # Set the class before we do anything else, so that it's set
-        # during the evaluation and can be inspected.
+        # Set the class before we evaluate the code, so that it's set during
+        # the evaluation and can be inspected.
         scope.compiler.class_set(self.classname, scope)
 
         # Now evaluate our code, yo.
diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb
index 8cebac8a8..2bf6c1882 100644
--- a/lib/puppet/parser/ast/node.rb
+++ b/lib/puppet/parser/ast/node.rb
@@ -5,34 +5,6 @@ require 'puppet/parser/ast/hostclass'
 class Puppet::Parser::AST::Node < Puppet::Parser::AST::HostClass
     @name = :node
 
-    # Evaluate the code associated with our node definition.
-    def evaluate_code(resource)
-        scope = resource.scope
-
-        # 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.evaluate_code(resource)
-        end
-
-        scope = scope.newscope(
-            :resource => resource,
-            :keyword => @keyword,
-            :source => self,
-            :namespace => "" # nodes are always in ""
-        )
-
-        # Mark our node name as a class, too, but strip it of the domain
-        # name.  Make the mark before we evaluate the code, so that it is
-        # marked within the code itself.
-        scope.compiler.class_set(self.classname, scope)
-
-        # And then evaluate our code if we have any
-        @code.safeevaluate(scope) if self.code
-
-        return scope
-    end
-
     def initialize(options)
         @parentclass = nil
         super
@@ -43,13 +15,19 @@ class Puppet::Parser::AST::Node < Puppet::Parser::AST::HostClass
         end
     end
 
+    def namespace
+        ""
+    end
+
     # Make sure node scopes are marked as such.
     def subscope(*args)
         scope = super
         scope.nodescope = true
+        scope
     end
 
     private
+
     # Search for the object matching our parent class.
     def find_parentclass
         @parser.findnode(parentclass)
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 26fdd3743..68c06e500 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -31,6 +31,7 @@ class Puppet::Parser::Compiler
 
     # Store a resource in our resource table.
     def add_resource(scope, resource)
+        # Note that this will fail if the resource is not unique.
         @catalog.add_resource(resource)
 
         # And in the resource graph.  At some point, this might supercede
@@ -48,10 +49,10 @@ class Puppet::Parser::Compiler
     # 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? or scope.nodescope?
+            if existing.nodescope? != scope.nodescope?
                 raise Puppet::ParseError, "Cannot have classes, nodes, or definitions with the same name"
             else
-                raise Puppet::DevError, "Somehow evaluated the same class twice"
+                raise Puppet::DevError, "Somehow evaluated %s %s twice" % [ existing.nodescope? ? "node" : "class", name]
             end
         end
         @class_scopes[name] = scope
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
index ea53b421a..c59748049 100644
--- a/lib/puppet/parser/resource/reference.rb
+++ b/lib/puppet/parser/resource/reference.rb
@@ -37,10 +37,14 @@ class Puppet::Parser::Resource::Reference < Puppet::ResourceReference
                 if self.title == :main
                     tmp = @scope.findclass("")
                 else
-                    tmp = @scope.findclass(self.title)
+                    unless tmp = @scope.findclass(self.title)
+                        fail Puppet::ParseError, "Could not find class '%s'" % self.title
+                    end
                 end
             when "Node": # look for node definitions
-                tmp = @scope.parser.nodes[self.title]
+                unless tmp = @scope.parser.nodes[self.title]
+                    fail Puppet::ParseError, "Could not find node '%s'" % self.title
+                end
             else # normal definitions
                 # We have to swap these variables around so the errors are right.
                 tmp = @scope.finddefine(self.type)