Fixing a weird bug that occurred because I was changing @parentclass in the AST stuff the first time it was called, from a string to a the actual instance of the parent. This worked fine as long as the parentclass was only called when parsing was complete, such as during evaluation, but if anything resulted in it being called earlier (e.g., attempting to add to the class during parsing), then things behaved, um, badly. This commit fixes the method so that the variable is not modified; there is now @parentclass and @parentobj.

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

lib/puppet/parser/ast/component.rb

@@ -21,6 +21,8 @@ class Puppet::Parser::AST
         # These are retrieved when looking up the superclass
         attr_accessor :name
 
+        attr_reader :parentclass
+
         def child_of?(klass)
             false
         end
@@ -132,10 +134,8 @@ class Puppet::Parser::AST
             end
         end
 
-        def parentclass
-            parentobj do |name|
-                @interp.findclass(namespace, name)
-            end
+        def find_parentclass
+            @interp.findclass(namespace, parentclass)
         end
 
         # Set our parent class, with a little check to avoid some potential
@@ -152,8 +152,8 @@ class Puppet::Parser::AST
         def parentobj
             if @parentclass
                 # Cache our result, since it should never change.
-                unless @parentclass.is_a?(AST::HostClass)
-                    unless tmp = yield(@parentclass)
+                unless defined?(@parentobj)
+                    unless tmp = find_parentclass
                         parsefail "Could not find %s %s" % [self.class.name, @parentclass]
                     end
 
@@ -161,9 +161,9 @@ class Puppet::Parser::AST
                         parsefail "Parent classes must have dissimilar names"
                     end
 
-                    @parentclass = tmp
+                    @parentobj = tmp
                 end
-                @parentclass
+                @parentobj
             else
                 nil
             end

lib/puppet/parser/ast/hostclass.rb

@@ -12,10 +12,10 @@ class Puppet::Parser::AST
         def child_of?(klass)
             return false unless self.parentclass
 
-            if klass == self.parentclass
+            if klass == self.parentobj
                 return true
             else
-                return self.parentclass.child_of?(klass)
+                return self.parentobj.child_of?(klass)
             end
         end
 
@@ -31,15 +31,11 @@ class Puppet::Parser::AST
             end
 
             pnames = nil
-            if @parentclass
-                if pklass = self.parentclass
-                    pklass.safeevaluate :scope => scope
+            if pklass = self.parentobj
+                pklass.safeevaluate :scope => scope
 
-                    scope = parent_scope(scope, pklass)
-                    pnames = scope.namespaces
-                else
-                    parsefail "Could not find class %s" % @parentclass
-                end
+                scope = parent_scope(scope, pklass)
+                pnames = scope.namespaces
             end
 
             unless hash[:nosubscope]

lib/puppet/parser/ast/node.rb

@@ -19,7 +19,7 @@ class Puppet::Parser::AST
 
             # 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.parentclass
+            if parent = self.parentobj
                 scope = parent.safeevaluate :scope => scope
             end
 
@@ -53,12 +53,10 @@ class Puppet::Parser::AST
             end
         end
 
-        def parentclass
-            parentobj do |name|
-                @interp.nodesearch(name)
-            end
-
-            @parentclass
+        private
+        # Search for the object matching our parent class.
+        def find_parentclass
+            @interp.nodesearch(parentclass)
         end
     end
 end

test/language/interpreter.rb

@@ -379,14 +379,14 @@ class TestInterpreter < Test::Unit::TestCase
 
         # Make sure trying to get the parentclass throws an error
         assert_raise(Puppet::ParseError) do
-            interp.nodesearch_code("simplenode").parentclass
+            interp.nodesearch_code("simplenode").parentobj
         end
 
         # Now define the parent node
         interp.newnode(:foo)
 
         # And make sure we get things back correctly
-        assert_equal("foo", interp.nodesearch_code("simplenode").parentclass.classname)
+        assert_equal("foo", interp.nodesearch_code("simplenode").parentobj.classname)
         assert_nil(interp.nodesearch_code("simplenode").code)
 
         # Now make sure that trying to redefine it throws an error.
@@ -402,7 +402,7 @@ class TestInterpreter < Test::Unit::TestCase
 
         names.each do |name|
             assert_equal(:yay, interp.nodesearch_code(name).code)
-            assert_equal("foo", interp.nodesearch_code(name).parentclass.name)
+            assert_equal("foo", interp.nodesearch_code(name).parentobj.name)
             # Now make sure that trying to redefine it throws an error.
             assert_raise(Puppet::ParseError) {
                 interp.newnode(name, {})
@@ -655,9 +655,11 @@ class TestInterpreter < Test::Unit::TestCase
             interp.newclass("sub", :parent => "base1")
         }
 
-        # Make sure we get the right parent class, and make sure it's an object.
-        assert_equal(interp.findclass("", "base1"),
+        # Make sure we get the right parent class, and make sure it's not an object.
+        assert_equal("base1",
                     interp.findclass("", "sub").parentclass)
+        assert_equal(interp.findclass("", "base1"),
+                    interp.findclass("", "sub").parentobj)
 
         # Now make sure we get a failure if we try to conflict.
         assert_raise(Puppet::ParseError) {
@@ -666,13 +668,13 @@ class TestInterpreter < Test::Unit::TestCase
 
         # Make sure that failure didn't screw us up in any way.
         assert_equal(interp.findclass("", "base1"),
-                    interp.findclass("", "sub").parentclass)
+                    interp.findclass("", "sub").parentobj)
         # But make sure we can create a class with a fq parent
         assert_nothing_raised {
             interp.newclass("another", :parent => "one::two::three")
         }
         assert_equal(interp.findclass("", "one::two::three"),
-                    interp.findclass("", "another").parentclass)
+                    interp.findclass("", "another").parentobj)
 
     end
 

test/language/parser.rb

@@ -397,7 +397,7 @@ file { "/tmp/yayness":
         }
         sub = interp.findclass("", "container::deep::sub")
         assert(sub, "Could not find sub")
-        assert_equal("base", sub.parentclass.classname)
+        assert_equal("base", sub.parentobj.classname)
         
         # Now try it with a parent class being a fq class
         assert_nothing_raised {
@@ -405,7 +405,7 @@ file { "/tmp/yayness":
         }
         sub = interp.findclass("", "container::one")
         assert(sub, "Could not find one")
-        assert_equal("container::deep::sub", sub.parentclass.classname)
+        assert_equal("container::deep::sub", sub.parentobj.classname)
         
         # Finally, try including a qualified class
         assert_nothing_raised("Could not include fully qualified class") {